Closed Bug 1472087 Opened 7 years ago Closed 7 years ago

DeCOMtaminate nsIDocShellLoadInfo

Categories

(Core :: DOM: Navigation, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: qdot, Assigned: qdot)

References

(Blocks 1 open bug)

Details

Attachments

(6 files, 1 obsolete file)

nsIDocShellLoadInfo isn't really used between JS and C++ (the only IDL references are as parameters, and the only JS use is of the loadHistory type), so we should be able to consolidate it to C++ only.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b558e4dcdd75e98e75ebb1a79795566bc54fbe38 Cleanup left: - Turn nsDocShellInfoLoadType into its own enum in nsDocShellLoadTypes - Turn nsDocShellInfoReferrerPolicy into its own enum in nsDocShellLoadTypes - Change nsDocShellLoadInfo getters/setters to webidl style instead of XPCOM style
Priority: -- → P2
Comment on attachment 8989506 [details] Bug ? - Remove redundant Docshell destroyed check; Wrong patch, not supposed to be in this set.
Attachment #8989506 - Attachment is obsolete: true
Attachment #8989506 - Flags: review?(nika)
Attachment #8989506 - Attachment is obsolete: false
Attachment #8989506 - Attachment is obsolete: true
Comment on attachment 8988970 [details] Bug 1472087 - deCOMtaminate nsIDocShellLoadInfo; https://reviewboard.mozilla.org/r/254084/#review261634 ::: docshell/base/nsDocShell.cpp:713 (Diff revision 2) > if (aLoadInfo) { > aLoadInfo->GetReferrer(getter_AddRefs(referrer)); > aLoadInfo->GetOriginalURI(getter_AddRefs(originalURI)); > GetMaybeResultPrincipalURI(aLoadInfo, resultPrincipalURI); > aLoadInfo->GetLoadReplace(&loadReplace); > - nsDocShellInfoLoadType lt = nsIDocShellLoadInfo::loadNormal; > + nsDocShellLoadInfo::nsDocShellInfoLoadType lt = nsDocShellLoadInfo::loadNormal; I think that, given this type is namespaced now, we can probably get away with calling it just 'LoadType'. ::: docshell/base/nsDocShellLoadInfo.h:23 (Diff revision 2) > > -class nsDocShellLoadInfo : public nsIDocShellLoadInfo > +class nsDocShellLoadInfo > { > public: > + typedef uint32_t nsDocShellInfoLoadType; > + typedef uint32_t nsDocShellInfoReferrerPolicy; both of these can probably be given more terse names, but come to think of it you might do that in one of the other... 6? patches. ::: toolkit/components/viewsource/content/viewSource-content.js:268 (Diff revision 2) > .createInstance(Ci.nsISHEntry); > shEntry.setURI(Services.io.newURI(viewSrcURL)); > shEntry.setTitle(viewSrcURL); > let systemPrincipal = Services.scriptSecurityManager.getSystemPrincipal(); > shEntry.triggeringPrincipal = systemPrincipal; > - shEntry.loadType = Ci.nsIDocShellLoadInfo.loadHistory; > + shEntry.loadType = 2; /* nsDocShellLoadInfo::loadHistory */ :-/ - Thanks for adding the comment there at least! ::: toolkit/modules/sessionstore/SessionHistory.jsm:346 (Diff revision 2) > > shEntry.setURI(Utils.makeURI(entry.url)); > shEntry.setTitle(entry.title || entry.url); > if (entry.subframe) > shEntry.setIsSubFrame(entry.subframe || false); > - shEntry.loadType = Ci.nsIDocShellLoadInfo.loadHistory; > + shEntry.loadType = 2; /* nsDocShellLoadInfo::loadHistory */ :-/
Attachment #8988970 - Flags: review?(nika) → review+
Attachment #8988971 - Flags: review?(nika) → review+
Attachment #8988972 - Flags: review?(nika) → review+
Comment on attachment 8989507 [details] Bug 1472087 - Convert nsDocShellLoadInfo to WebIDL style; https://reviewboard.mozilla.org/r/254542/#review261640 ::: commit-message-1b750:4 (Diff revision 1) > +Bug 1472087 - Convert nsDocShellLoadInfo to WebIDL style; r?nika > + > +While nsDocShellLoadInfo isn't represented by WebIDL (because we don't > +need it in JS currently), make the getter/setter interface look Is the plan to eventually also expose nsDocShellLoadInfo to JS so that we can convert all load entry points in nsDocShell to using it? ::: docshell/base/nsDocShellLoadInfo.h:22 (Diff revision 1) > class nsIDocShell; > > class nsDocShellLoadInfo > { > public: > typedef uint32_t nsDocShellInfoLoadType; Can we kill this typedef? We have an enum type now. ::: docshell/base/nsDocShellLoadInfo.h:157 (Diff revision 1) > bool mInheritPrincipal; > bool mPrincipalIsExplicit; > bool mForceAllowDataURI; > bool mOriginalFrameSrc; > bool mSendReferrer; > - nsDocShellInfoReferrerPolicy mReferrerPolicy; > + uint32_t mReferrerPolicy; Could/should we use the 'mozilla::net::ReferrerPolicy' enum for this to make it more clear? ::: docshell/base/nsDocShellLoadInfo.cpp:159 (Diff revision 1) > { > mOriginalFrameSrc = aOriginalFrameSrc; > - return NS_OK; > } > > -NS_IMETHODIMP > +nsDocShellLoadInfo::nsDocShellInfoLoadType Can we make this nsDocShellLoadInfo::LoadType now - we have the enum :-) ::: docshell/base/nsDocShellLoadInfo.cpp:166 (Diff revision 1) > - *aLoadType = mLoadType; > - return NS_OK; > } > > -NS_IMETHODIMP > -nsDocShellLoadInfo::SetLoadType(nsDocShellInfoLoadType aLoadType) > +void > +nsDocShellLoadInfo::SetLoadType(nsDocShellLoadInfo::nsDocShellInfoLoadType aLoadType) Same here
Attachment #8989507 - Flags: review?(nika) → review+
Comment on attachment 8989508 [details] Bug 1472087 - Remove nsDocShellLoadInfo::LoadTypes; https://reviewboard.mozilla.org/r/254544/#review261646 ::: toolkit/components/viewsource/content/viewSource-content.js:268 (Diff revision 1) > .createInstance(Ci.nsISHEntry); > shEntry.setURI(Services.io.newURI(viewSrcURL)); > shEntry.setTitle(viewSrcURL); > let systemPrincipal = Services.scriptSecurityManager.getSystemPrincipal(); > shEntry.triggeringPrincipal = systemPrincipal; > - shEntry.loadType = 2; /* nsDocShellLoadInfo::loadHistory */ > + shEntry.loadType = 4; /* nsDocShellLoadTypes LOAD_HISTORY */ This is a touch more nasty now :-/... Maybe even add a method to nsISHistory like setAsHistoryLoad()? D:
Attachment #8989508 - Flags: review?(nika) → review+
Comment on attachment 8989509 [details] Bug 1472087 - Readd comments from nsIDocShellLoadInfo; https://reviewboard.mozilla.org/r/254546/#review261648 ::: docshell/base/nsDocShellLoadInfo.h:21 (Diff revision 1) > class nsISHEntry; > class nsIURI; > class nsIDocShell; > > +/** > + * nsDocShellLoadInfo defines an interface for specifying setup information used nsDocShellLoadInfo isn't really an interface anymore :-P
Attachment #8989509 - Flags: review?(nika) → review+
Pushed by kmachulis@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e9df2dcfe09f deCOMtaminate nsIDocShellLoadInfo; r=nika https://hg.mozilla.org/integration/mozilla-inbound/rev/ffbed43e8c35 Remove nsIDocShellLoadInfo.idl; r=nika https://hg.mozilla.org/integration/mozilla-inbound/rev/74c808bf7da8 Remove nsDocShell::CreateLoadInfo; r=nika https://hg.mozilla.org/integration/mozilla-inbound/rev/f0289eb6a02d Convert nsDocShellLoadInfo to WebIDL style; r=nika https://hg.mozilla.org/integration/mozilla-inbound/rev/fd86e6f12106 Remove nsDocShellLoadInfo::LoadTypes; r=nika https://hg.mozilla.org/integration/mozilla-inbound/rev/8d9d71567046 Readd comments from nsIDocShellLoadInfo; r=nika
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: